fix: disable theme icon loading for battery display#390
Merged
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis PR disables loading battery icons from the system theme and always uses the bundled SVG battery icons as the primary source to ensure consistent rendering in the dock across all battery states. Sequence diagram for updated battery icon refresh logicsequenceDiagram
actor DockUser
participant PowerManager
participant PowerApplet
participant QSvgRenderer
participant BatteryIconWidget
DockUser->>PowerManager: Change battery state
PowerManager-->>PowerApplet: batteryStateChanged(level, charging)
PowerApplet->>PowerApplet: compute iconName and fallbackPath
PowerApplet->>QSvgRenderer: QSvgRenderer(fallbackPath)
QSvgRenderer-->>PowerApplet: renderer (validated)
PowerApplet->>BatteryIconWidget: setPixmap(renderedSvgFrom(fallbackPath))
BatteryIconWidget-->>DockUser: Updated battery icon displayed
Class diagram for PowerApplet battery icon rendering dependenciesclassDiagram
class PowerApplet {
- m_batteryIcon : QLabel*
+ refreshBatteryIcon(icon : QString) void
}
class QLabel {
+ setPixmap(pixmap : QPixmap) void
+ size() QSize
}
class QSvgRenderer {
+ QSvgRenderer(filePath : QString)
+ isValid() bool
+ render(painter : QPainter) void
}
PowerApplet --> QLabel : updates
PowerApplet --> QSvgRenderer : uses built_in_svg_icons
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- Since the theme icon loading path is intentionally disabled, consider removing the commented-out block entirely or replacing it with a clear TODO explaining under what conditions it should be re-enabled to avoid leaving dead code.
- The new comment only states that the plugin does not follow theme changes; if behavior differs between light and dark modes or between themes using different battery icon designs, it may be worth clarifying this in the comment to make the rationale for forcing built-in SVGs more explicit for future maintainers.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Since the theme icon loading path is intentionally disabled, consider removing the commented-out block entirely or replacing it with a clear TODO explaining under what conditions it should be re-enabled to avoid leaving dead code.
- The new comment only states that the plugin does not follow theme changes; if behavior differs between light and dark modes or between themes using different battery icon designs, it may be worth clarifying this in the comment to make the rationale for forcing built-in SVGs more explicit for future maintainers.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Removed the code that attempts to load battery icons from system theme and now directly uses the built-in SVG icons. This change was necessary because the plugin currently does not support theme changes, and the theme icon loading was causing inconsistent battery icon display. The fallback mechanism now becomes the primary method for loading battery icons. Log: Fixed inconsistent battery icon display in dock Influence: 1. Verify battery icons display correctly in different states (charging, low battery, etc.) 2. Test battery percentage changes and corresponding icon updates 3. Check that all battery level icons are properly rendered 4. Verify no missing or broken battery icons in dock 5. Test with different battery levels and charging states fix: 禁用主题图标加载，使用内置电池图标 移除了从系统主题加载电池图标的代码，现在直接使用内置的SVG图标。此变更是 必要的，因为插件目前不支持主题变化，主题图标加载会导致电池图标显示不一 致。后备机制现在成为加载电池图标的主要方法。 Log: 修复了dock中电池图标显示不一致的问题 Influence: 1. 验证不同状态下的电池图标显示正确（充电、低电量等） 2. 测试电池百分比变化及对应的图标更新 3. 检查所有电池电量图标是否正确渲染 4. 验证dock中没有缺失或损坏的电池图标 5. 测试不同电量水平和充电状态下的显示 PMS: BUG-342033
wjyrich
force-pushed
the
fix-battery-popup
branch
from
0773fa3 to
b73fa49
Compare
mhduiy
approved these changes
Nov 26, 2025
deepin pr auto review我来对这段代码变更进行审查：
潜在问题：
改进建议：
// 可以添加一个配置项来控制是否使用主题图标
if (m_useThemeIcons) {
QIcon themeIcon = QIcon::fromTheme(iconName);
if (!themeIcon.isNull()) {
QPixmap pixmap = themeIcon.pixmap(m_batteryIcon->size());
m_batteryIcon->setPixmap(pixmap);
return;
}
}
QSvgRenderer renderer(fallbackPath);
if (!renderer.isValid()) {
qWarning() << "Failed to load battery icon:" << fallbackPath;
// 可以设置一个默认的错误图标
m_batteryIcon->setPixmap(QPixmap(":/batteryicons/batteryicons/error.svg"));
return;
}
static QCache<QString, QPixmap> iconCache(10); // 缓存最近使用的10个图标
auto cachedPixmap = iconCache.object(fallbackPath);
if (cachedPixmap) {
m_batteryIcon->setPixmap(*cachedPixmap);
return;
}
private:
bool m_useThemeIcons = false; // 可通过配置文件控制
QCache<QString, QPixmap> m_iconCache;这些改进可以让代码更加健壮和灵活，同时保持良好的性能。 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: mhduiy, wjyrich The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Contributor
Author
|
/forcemerge |
|
This pr force merged! (status: behind) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Removed the code that attempts to load battery icons from system theme and now directly uses the built-in SVG icons. This change was necessary because the plugin currently does not support theme changes, and the theme icon loading was causing inconsistent battery icon display. The fallback mechanism now becomes the primary method for loading battery icons.
Log: Fixed inconsistent battery icon display in dock
Influence:
fix: 禁用主题图标加载，使用内置电池图标
移除了从系统主题加载电池图标的代码，现在直接使用内置的SVG图标。此变更是
必要的，因为插件目前不支持主题变化，主题图标加载会导致电池图标显示不一
致。后备机制现在成为加载电池图标的主要方法。
Log: 修复了dock中电池图标显示不一致的问题
Influence:
PMS: BUG-342033
Summary by Sourcery
Bug Fixes: